fix(garm): treat 401 from controller-info as initialised#283
Merged
Conversation
is_initialized() probes GET /controller-info without auth. GARM gates that endpoint on two middlewares: a 409 until first-run has created the admin user, then JWT auth. On an already-initialised GARM (e.g. after a charm upgrade with persisted PostgreSQL), the unauthenticated probe passes the init gate and hits the auth gate, returning 401, which was being raised as a fatal GarmApiError and failing the config-changed hook. 401 now means initialised, same as 200; only 409 means not-yet-initialised.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes the GARM charm’s first-run initialization probe so that an unauthenticated GET /api/v1/controller-info returning 401 Unauthorized is treated as “already initialized” (same as 200), rather than raising and failing hooks (notably config-changed) on previously-initialized controllers.
Changes:
- Update
GarmApiClient.is_initialized()to treatApiException(status=401)as initialized (True), while keeping409as not-initialized (False). - Extend unit tests to cover the new
401 → Truebehavior. - Document the user-facing fix in
docs/changelog.md.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
charms/garm/src/garm_api.py |
Adjusts initialization detection logic to treat 401 from controller-info as initialized. |
charms/garm/tests/unit/test_garm_api.py |
Adds a 401 case to the is_initialized() parametrized test table. |
docs/changelog.md |
Notes the hook-failure fix in the changelog entry for 2026-07-13. |
Comments suppressed due to low confidence (1)
charms/garm/src/garm_api.py:115
wait_for_ready()'s docstring still saysTrue (200)even thoughis_initialized()now also returnsTrueon401. This is now misleading for future maintainers reading the readiness logic.
raise GarmApiError(
f"Unexpected response from GARM controller-info ({exc.status}): {exc.body}"
) from exc
except urllib3.exceptions.HTTPError as exc:
raise GarmConnectionError(f"GARM connection error: {exc}") from exc
cbartz
requested review from
florentianayuwono,
javierdelapuente,
weiiwang01,
yanksyoon and
yhaliaw
as code owners
weiiwang01
approved these changes
Jul 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
Fixes
GarmApiClient.is_initialized()so it treats a401from GARM'scontroller-infoendpoint as "initialised" (same as200), instead of raising and failing the charm hook.Why we need it
The first-run check probes
GET /api/v1/controller-infowith a deliberately unauthenticated client. GARM puts that endpoint behind two middlewares: an init-required gate (returns409until first-run creates the admin user) and JWT auth (returns401when there's no token). The code only understood200→ initialised and409→ not-initialised, and raised on anything else.On an already-initialised GARM (e.g. after a charm upgrade with persisted PostgreSQL), the init gate passes and the tokenless probe hits the auth gate →
401, which was wrongly treated as fatal and failed theconfig-changedhook. It only appeared to "self-heal" because Juju's retry found the workload config hash already applied and short-circuitedrestart()before re-reaching the probe. The bug is latent for any config/charm change that rewrites the workload config on an initialised controller.A
401unambiguously means the request got past the init gate, so it means initialised — the probe never needs a token to answer that question.Test plan
test_is_initializedparametrize table incharms/garm/tests/unit/test_garm_api.pywith a401 → Truecase.tox -c tox.toml -e unit→ 229 passed;-e lintand-e static(pyright: 0 errors) clean.Review focus
401on the unauthenticatedcontroller-infoprobe reliably means "initialised" (init gate passed, auth gate rejected).wait_for_ready()is intentionally unchanged —200/401/409all confirm the HTTP server is up, and it still retries only on connection-level errors.Potential breaking changes / new dependencies
None. No new dependencies, APIs, or workflow changes.
Checklist
CONTRIBUTING.mdhas been updated upon changes to the contribution/development process (e.g. changes to the way tests are run) — n/a, no process changedocs/changelog.mdwith user-relevant changesterraform fmtpasses andtflintreports no errors — n/aAGENTS.md— n/a.copilot-collections.yamlor.github/instructions/: I re-checked whether theAGENTS.md"12-factor divergences" guidance still matches upstream — n/a